Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add the cleanupexamples tool #215

Merged
merged 5 commits into from
Sep 17, 2024
Merged

Add the cleanupexamples tool #215

merged 5 commits into from
Sep 17, 2024

Conversation

turkenf
Copy link
Contributor

@turkenf turkenf commented Aug 27, 2024 •
edited
Loading

Description of your changes

We publish some uptest specific codes in the example manifests to the Upbound marketplace. For example, we add the annotation upjet.upbound.io/manual-intervention to skip uptest in resources that require manual intervention, or we add ${Rand.RFC1123Subdomain} to generate a random value in fields that require a unique string.

These uptest specific codes and placeholders can be confusing for users. See as an example: crossplane-contrib/provider-upjet-aws#988

This PR adds the cleanupexamples tool, which removes the above-mentioned confusion.

I have:

  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

For now, this tool has been tested manually in my local environment. We will do more testing when we integrate the code into the build process.

  • Below is an example input YAML:
apiVersion: apigateway.aws.upbound.io/v1beta1
kind: BasePathMapping
metadata:
  annotations:
    crossplane.io/external-name: test name
    upjet.upbound.io/manual-intervention: "The BasePathMapping resource needs a DomainName and DomainName resource needs valid certificates."
    uptest.upbound.io/timeout: "3600" # one hour timeout
    uptest.upbound.io/disable-import: "true"
    uptest.upbound.io/pre-delete-hook: testhooks/delete
  labels:
    testing.upbound.io/example-name: domainname
  name: example-${Rand.RFC1123Subdomain}
spec:
  forProvider:
    apiId: demo
    domain: example
    region: us-west-1
  • and the output YAML generated with the tool:
apiVersion: apigateway.aws.upbound.io/v1beta1
kind: BasePathMapping
metadata:
  annotations:
    crossplane.io/external-name: test name
  labels:
    testing.upbound.io/example-name: domainname
  name: example-random
spec:
  forProvider:
    apiId: demo
    domain: example
    region: us-west-1

@turkenf
Add the cleanupexamples tool
6f881a9 
Signed-off-by: Fatih Türken <[email protected]>
@turkenf turkenf force-pushed the cleanup-examples branch 3 times, most recently from 0723e84 to cad2a3c Compare August 28, 2024 15:57
@turkenf
Fix linter issues
fd90fee 
Signed-off-by: Fatih Türken <[email protected]>
@turkenf turkenf marked this pull request as ready for review August 29, 2024 09:26
mergenci
mergenci approved these changes Sep 11, 2024
View reviewed changes
Copy link
Contributor

@mergenci mergenci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @turkenf, especially for providing unit tests as well 😎 Please find my comments below. I'm approving the PR, in case you would like to merge soon. I would be happy to see another review though, just to have another point of view.

cmd/cleanupexamples/main.go Outdated Show resolved Hide resolved
cmd/cleanupexamples/main.go Outdated Show resolved Hide resolved
@turkenf
Address review comments
dff19a8 
Signed-off-by: Fatih Türken <[email protected]>
sergenyalcin
sergenyalcin approved these changes Sep 16, 2024
View reviewed changes
Copy link
Member

@sergenyalcin sergenyalcin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @turkenf for your great work here. LGTM!

cmd/cleanupexamples/main.go Show resolved Hide resolved
@turkenf turkenf merged commit 44c3fe8 into upbound:main Sep 17, 2024
6 checks passed
@turkenf turkenf deleted the cleanup-examples branch September 17, 2024 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sergenyalcin sergenyalcin sergenyalcin approved these changes

@mergenci mergenci mergenci approved these changes

@ulucinar ulucinar Awaiting requested review from ulucinar ulucinar is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@turkenf @mergenci @sergenyalcin